Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add refactoring to replace ! with .Value #11227

Merged
merged 5 commits into from
Mar 19, 2021

Conversation

cartermp
Copy link
Contributor

@cartermp cartermp commented Mar 12, 2021

This should help with: fsharp/fslang-suggestions#569

refactor-deref

refactor-deref2

@@ -46,6 +46,9 @@ type public FSharpParseFileResults =
/// </summary>
member TryRangeOfRefCellDereferenceContainingPos: expressionPos: pos -> range option

/// Gets the range of an expression being dereferenced. For `!expr`, gives the range of `expr`
member TryRangeOfExpressionBeingDereferencedContainingPos: expressionPos: pos -> range option
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more efficient approach is to unify this with TryRangeOfRefCellDereferenceContainingPos. This would be a breaking change, but we're already doing several breaks in FCS 40.

Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice,

Thanks.

member _.VisitExpr(_, _, defaultTraverse, expr) =
match expr with
| SynExpr.App(_, false, SynExpr.Ident funcIdent, expr, _) ->
if funcIdent.idText = "op_Dereference" && rangeContainsPos expr.Range expressionPos then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but perhaps we should consider an enum for id constants.

@cartermp cartermp merged commit db9805c into dotnet:main Mar 19, 2021
@cartermp cartermp deleted the deref-value-refactor branch March 19, 2021 23:01
@runfoapp runfoapp bot mentioned this pull request Mar 22, 2021
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants